fix: fullpath handling #6404
Merged
fix: fullpath handling #6404
+961
−358
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughThis PR normalizes route path handling (trailing-slash semantics), fixes pathless-layout fullPath inference (root becomes '/'), enforces uniqueness/validation for empty route paths, updates route map construction with index-route precedence, adjusts route initialization to trim _to, and adds tests and snapshots covering the changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Gen as Generator
participant Infer as inferPath / inferFullPath
participant Map as createRouteNodesByFullPath/To
participant Check as checkRouteFullPathUniqueness
participant Route as Route Init
Gen->>Infer: compute cleanedPath(route.path)
Infer-->>Gen: cleanedPath ('' or '/...' normalized)
Gen->>Map: iterate nodes
Map->>Infer: inferFullPath(node)
Infer-->>Map: fullPath ('' → '/', index retains trailing '/')
Map->>Map: if fullPath '/' and exists -> shouldPreferIndexRoute?
alt prefer existing index
Map-->>Map: skip or replace based on precedence
else
Map-->>Map: insert route
end
Map-->>Gen: built route map
Gen->>Check: validate route paths
Check->>Check: if routePath === '' -> throw detailed error
Check-->>Gen: validated or error
Gen->>Route: create Route(fullPath)
Route->>Route: _to = trimPathRight(fullPath)
Route-->>Gen: route instance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 15m 14s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 1m 42s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-01-17 16:15:49 UTC
SeanCassiere
approved these changes
Jan 17, 2026
Member
SeanCassiere
left a comment
SeanCassiere
left a comment
There was a problem hiding this comment.
It always comes back to the generator 😩
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@packages/router-generator/tests/generator/nested-verboseFileRoutes-false/routes/_pathlessLayout.tsx`:
- Around line 1-4: The file defines Route using createFileRoute but never
imports it; add the missing import for createFileRoute from
'@tanstack/react-router' alongside the existing Outlet import so the
createFileRoute symbol is available when constructing the Route component
(ensure the import statement includes createFileRoute and retains Outlet to
avoid the runtime error in the Route definition).
In
`@packages/router-generator/tests/generator/nested-verboseFileRoutes-true/tests.test-d.ts`:
- Around line 13-16: Reorder the imports in tests.test-d.ts so the type-only
import from '@tanstack/react-router' comes after the generated module import;
specifically, move "import type { FileRoutesByPath, MakeRouteMatch } from
'@tanstack/react-router'" to below "import { routeTree } from './routeTree.gen'"
(keeping "import type { FileRouteTypes } from './routeTree.gen'" where
appropriate) so ESLint's import ordering rule is satisfied while preserving the
existing references to FileRoutesByPath, MakeRouteMatch, FileRouteTypes,
expectTypeOf, and test.
🧹 Nitpick comments (1)
packages/router-generator/tests/generator/nested-verboseFileRoutes-false/tests.test-d.ts (1)
13-16: Import order: move type import after value imports.ESLint flags that the type import from
@tanstack/react-routershould occur after the import from./routeTree.gen.♻️ Proposed fix
import { Link, createRouter, redirect, useLoaderData, useLoaderDeps, useMatch, useNavigate, useParams, useRouteContext, useSearch, } from '@tanstack/react-router' -import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router' import { expectTypeOf, test } from 'vitest' import { routeTree } from './routeTree.gen' import type { FileRouteTypes } from './routeTree.gen' +import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router'
Comment on lines
+1
to
+4
| import { Outlet } from '@tanstack/react-router' | ||
| export const Route = createFileRoute({ | ||
| component: () => <Outlet />, | ||
| }) |
Contributor
There was a problem hiding this comment.
Missing createFileRoute import.
The file uses createFileRoute on line 2 but only imports Outlet from @tanstack/react-router. This will cause a runtime error.
🐛 Proposed fix
-import { Outlet } from '@tanstack/react-router'
+import { createFileRoute, Outlet } from '@tanstack/react-router'
export const Route = createFileRoute({
component: () => <Outlet />,
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { Outlet } from '@tanstack/react-router' | |
| export const Route = createFileRoute({ | |
| component: () => <Outlet />, | |
| }) | |
| import { createFileRoute, Outlet } from '@tanstack/react-router' | |
| export const Route = createFileRoute({ | |
| component: () => <Outlet />, | |
| }) |
🤖 Prompt for AI Agents
In
`@packages/router-generator/tests/generator/nested-verboseFileRoutes-false/routes/_pathlessLayout.tsx`
around lines 1 - 4, The file defines Route using createFileRoute but never
imports it; add the missing import for createFileRoute from
'@tanstack/react-router' alongside the existing Outlet import so the
createFileRoute symbol is available when constructing the Route component
(ensure the import statement includes createFileRoute and retains Outlet to
avoid the runtime error in the Route definition).
Comment on lines
+13
to
+16
| import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router' | ||
| import { expectTypeOf, test } from 'vitest' | ||
| import { routeTree } from './routeTree.gen' | ||
| import type { MakeRouteMatch } from '@tanstack/react-router' | ||
| import type { FileRouteTypes } from './routeTree.gen' |
Contributor
There was a problem hiding this comment.
Fix import ordering flagged by ESLint.
The static analysis tool indicates the type import from @tanstack/react-router should occur after the import of ./routeTree.gen.
Proposed fix
-import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router'
import { expectTypeOf, test } from 'vitest'
import { routeTree } from './routeTree.gen'
import type { FileRouteTypes } from './routeTree.gen'
+import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router' | |
| import { expectTypeOf, test } from 'vitest' | |
| import { routeTree } from './routeTree.gen' | |
| import type { MakeRouteMatch } from '@tanstack/react-router' | |
| import type { FileRouteTypes } from './routeTree.gen' | |
| import { expectTypeOf, test } from 'vitest' | |
| import { routeTree } from './routeTree.gen' | |
| import type { FileRouteTypes } from './routeTree.gen' | |
| import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router' |
🧰 Tools
🪛 ESLint
[error] 13-13: @tanstack/react-router type import should occur after import of ./routeTree.gen
(import/order)
🤖 Prompt for AI Agents
In
`@packages/router-generator/tests/generator/nested-verboseFileRoutes-true/tests.test-d.ts`
around lines 13 - 16, Reorder the imports in tests.test-d.ts so the type-only
import from '@tanstack/react-router' comes after the generated module import;
specifically, move "import type { FileRoutesByPath, MakeRouteMatch } from
'@tanstack/react-router'" to below "import { routeTree } from './routeTree.gen'"
(keeping "import type { FileRouteTypes } from './routeTree.gen'" where
appropriate) so ESLint's import ordering rule is satisfied while preserving the
existing references to FileRoutesByPath, MakeRouteMatch, FileRouteTypes,
expectTypeOf, and test.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@packages/router-generator/tests/generator/nested-verboseFileRoutes-false/tests.test-d.ts`:
- Around line 13-16: The import ordering violates the import/order rule; reorder
imports so the local module import "routeTree" and its type "FileRouteTypes"
(from './routeTree.gen') come before external library imports, and ensure
type-only imports use "import type" for FileRouteTypes, FileRoutesByPath, and
MakeRouteMatch; specifically move the line importing FileRouteTypes to directly
after "import { routeTree } from './routeTree.gen'" and place the external
imports (e.g., from '@tanstack/react-router' and 'vitest') after the local
imports to satisfy linting.
♻️ Duplicate comments (1)
packages/router-generator/tests/generator/nested-verboseFileRoutes-true/tests.test-d.ts (1)
13-16: Fix import ordering flagged by ESLint.The static analysis tool indicates the type import from
@tanstack/react-routershould occur after the import of./routeTree.gen.Proposed fix
-import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router' import { expectTypeOf, test } from 'vitest' import { routeTree } from './routeTree.gen' import type { FileRouteTypes } from './routeTree.gen' +import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router'
Comment on lines
+13
to
+16
| import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router' | ||
| import { expectTypeOf, test } from 'vitest' | ||
| import { routeTree } from './routeTree.gen' | ||
| import type { MakeRouteMatch } from '@tanstack/react-router' | ||
| import type { FileRouteTypes } from './routeTree.gen' |
Contributor
There was a problem hiding this comment.
Fix import ordering to satisfy import/order.
Static analysis reports a lint error; move the type-only import after the local routeTree.gen import.
Proposed fix
-import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router'
import { expectTypeOf, test } from 'vitest'
import { routeTree } from './routeTree.gen'
import type { FileRouteTypes } from './routeTree.gen'
+import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router' | |
| import { expectTypeOf, test } from 'vitest' | |
| import { routeTree } from './routeTree.gen' | |
| import type { MakeRouteMatch } from '@tanstack/react-router' | |
| import type { FileRouteTypes } from './routeTree.gen' | |
| import { expectTypeOf, test } from 'vitest' | |
| import { routeTree } from './routeTree.gen' | |
| import type { FileRouteTypes } from './routeTree.gen' | |
| import type { FileRoutesByPath, MakeRouteMatch } from '@tanstack/react-router' |
🧰 Tools
🪛 ESLint
[error] 13-13: @tanstack/react-router type import should occur after import of ./routeTree.gen
(import/order)
🤖 Prompt for AI Agents
In
`@packages/router-generator/tests/generator/nested-verboseFileRoutes-false/tests.test-d.ts`
around lines 13 - 16, The import ordering violates the import/order rule;
reorder imports so the local module import "routeTree" and its type
"FileRouteTypes" (from './routeTree.gen') come before external library imports,
and ensure type-only imports use "import type" for FileRouteTypes,
FileRoutesByPath, and MakeRouteMatch; specifically move the line importing
FileRouteTypes to directly after "import { routeTree } from './routeTree.gen'"
and place the external imports (e.g., from '@tanstack/react-router' and
'vitest') after the local imports to satisfy linting.
schiller-manuel
force-pushed
the
fix-fullpath
branch
from
fbf9776 to
053efe5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #6403
fixes #2675
fixes #4892
fixes #4804
fixes #3005
Summary by CodeRabbit
Bug Fixes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.